Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Monitoring] Fixed server response errors #63181

Merged
merged 6 commits into from
Apr 17, 2020
Merged

[Monitoring] Fixed server response errors #63181

merged 6 commits into from
Apr 17, 2020

Conversation

igoristic
Copy link
Contributor

Resolves #62843

This was caused because we weren't try/catching our main resolve/reject state

Testing

  1. Create a user with an insufficient role to enable monitoring
  2. Go to Kibana > Stack Monitoring > Click on Or, set up with self monitoring
  3. Notice the correct error type/statusCode
  4. The same errors can also be visible in the browser's console

@igoristic igoristic added bug Fixes for quality problems that affect the customer experience Team:Monitoring Stack Monitoring team v8.0.0 v7.0.1 labels Apr 9, 2020
@igoristic igoristic requested a review from a team April 9, 2020 19:47
@elasticmachine
Copy link
Contributor

Pinging @elastic/stack-monitoring (Team:Monitoring)

@igoristic
Copy link
Contributor Author

@elasticmachine merge upstream

chrisronline
chrisronline approved these changes Apr 10, 2020
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, nice work!

chrisronline
chrisronline suggested changes Apr 10, 2020
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hold up. After using this PR some more, I'm seeing things like this in the logs:

server    log   [14:04:28.628] [error][monitoring][monitoring][plugins] { Error: Unable to find the cluster in the selected time range. UUID: ECEBF2VPQuCF9tbBKaLqXQ
    at getClustersFromRequest (/Users/chris/dev/repos/kibana/x-pack/plugins/monitoring/server/lib/cluster/get_clusters_from_request.js:83:13)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  data: null,
  isBoom: true,
  isServer: false,
  output:
   { statusCode: 404,
     payload:
      { statusCode: 404,
        error: 'Not Found',
        message:
         'Unable to find the cluster in the selected time range. UUID: ECEBF2VPQuCF9tbBKaLqXQ' },
     headers: {} },
  reformat: [Function],
  typeof: [Function: notFound] }
server    log   [14:04:28.628] [error][http] { AssertionError [ERR_ASSERTION]: Cannot wrap an error
    at new AssertionError (internal/assert.js:269:11)
    at Object.exports.assert (/Users/chris/dev/repos/kibana/node_modules/hoek/lib/index.js:736:11)
    at internals.Toolkit.response (/Users/chris/dev/repos/kibana/node_modules/hapi/lib/toolkit.js:145:14)
    at HapiResponseAdapter.toSuccess (/Users/chris/dev/repos/kibana/src/core/server/http/router/response_adapter.ts:94:8)
    at HapiResponseAdapter.toHapiResponse (/Users/chris/dev/repos/kibana/src/core/server/http/router/response_adapter.ts:82:19)
    at HapiResponseAdapter.handle (/Users/chris/dev/repos/kibana/src/core/server/http/router/response_adapter.ts:74:17)
    at Router.handle (/Users/chris/dev/repos/kibana/src/core/server/http/router/router.ts:265:34)
    at process._tickCallback (internal/process/next_tick.js:68:7)
  generatedMessage: false,
  name: 'AssertionError [ERR_ASSERTION]',
  code: 'ERR_ASSERTION',
  actual: false,
  expected: true,
  operator: '==' }

@igoristic
Copy link
Contributor Author

Hold up. After using this PR some more, I'm seeing things like this in the logs...

Good find! Looking into this now

chrisronline
chrisronline approved these changes Apr 15, 2020
View reviewed changes
Copy link
Contributor

@chrisronline chrisronline left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@igoristic
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@igoristic igoristic merged commit dacc95f into elastic:master Apr 17, 2020
This was referenced Apr 17, 2020
Merged
Merged
igoristic added a commit to igoristic/kibana that referenced this pull request Apr 17, 2020
@igoristic @elasticmachine
[Monitoring] Fixed server response errors (elastic#63181)
1d51019 
* Fixed server response errors

* Fixed async error

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
igoristic added a commit to igoristic/kibana that referenced this pull request Apr 17, 2020
@igoristic @elasticmachine
[Monitoring] Fixed server response errors (elastic#63181)
b8ca181 
* Fixed server response errors

* Fixed async error

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Apr 17, 2020
@gmmorris
Merge branch 'master' into alerting/es-query
34e0612 
* master: (40 commits)
  [APM]Upgrade apm-rum agent to latest version to fix full page reload (elastic#63723)
  add deprecation warning for legacy 3rd party plugins (elastic#62401)
  Migrate timelion vis (elastic#62819)
  Replacebad scope link with actual values (elastic#63444)
  Index pattern management UI -> TypeScript and New Platform Ready (create_index_pattern_wizard) (elastic#63111)
  [SIEM] Threat hunting enhancements: Filter for/out value, Show top field, Copy to Clipboard, Draggable chart legends (elastic#61207)
  [Maps] fix term join agg key collision (elastic#63324)
  [Ingest] Fix agent config key sorting (elastic#63488)
  [Monitoring] Fixed server response errors (elastic#63181)
  update elastic charts to 18.3.0 (elastic#63732)
  Start services (elastic#63720)
  [APM] Encode spaces when creating ML job (elastic#63683)
  Uptime 7.7 docs (elastic#62228)
  [DOCS] Updates remote cluster and ccr docs (elastic#63517)
  [Maps] Add 3rd party vector tile support (elastic#62084)
  [Endpoint][EPM] Retrieve Index Pattern from Ingest Manager (elastic#63016)
  [Endpoint] Host Details Policy Response Panel (elastic#63518)
  [Uptime] Certificate expiration threshold settings (elastic#63682)
  Refactor saved object types to use `namespaceType` (elastic#63217)
  [SIEM][CASE] Create comments sequentially (elastic#63692)
  ...
igoristic added a commit that referenced this pull request Apr 17, 2020
@igoristic @elasticmachine
[Monitoring] Fixed server response errors (#63181) (#63811)
a1039fc 
* Fixed server response errors

* Fixed async error

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
igoristic added a commit that referenced this pull request Apr 17, 2020
@igoristic @elasticmachine
[Monitoring] Fixed server response errors (#63181) (#63812)
70021fe 
* Fixed server response errors

* Fixed async error

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@igoristic
Copy link
Contributor Author

Backport:
7.x: b8ca181
7.7: 1d51019

@igoristic igoristic deleted the fix_errors branch April 17, 2020 19:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisronline chrisronline chrisronline approved these changes

Assignees
No one assigned
Labels
backported bug Fixes for quality problems that affect the customer experience release_note:fix Team:Monitoring Stack Monitoring team v7.7.1 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Monitoring] Propagate the correct error and statusCode when a request fails
4 participants
@igoristic @elasticmachine @kibanamachine @chrisronline